Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bump to v31 #246

Merged
merged 45 commits into from
Feb 1, 2024
Merged

Bump to v31 #246

merged 45 commits into from
Feb 1, 2024

Conversation

BSd3v
Copy link
Collaborator

@BSd3v BSd3v commented Oct 16, 2023

Reopening to go onto main since dev was deleted

BSd3v and others added 4 commits July 25, 2023 12:17
- fixing breaking changes
…object of objects that need to be parsed. This may be predetermined or no, it needs to loop through the keys.
@alexcjohnson alexcjohnson changed the title Bump to v30 Bump to v31 Nov 29, 2023
@AnnMarieW
Copy link
Collaborator

AnnMarieW commented Dec 2, 2023

Update - this example works now 🎉

I'm trying to get This example from the AG Grid docs working, but I'm not sure if I'm defining it wrong or if dataTypeDefinitions prop needs to have function enabled.


import dash_ag_grid as dag
from dash import Dash, html
import pandas as pd


app = Dash(__name__)

df = pd.read_csv(
    "https://raw.githubusercontent.com/plotly/datasets/master/ag-grid/olympic-winners.csv"
)

columnDefs = [
    {"field": 'athlete'},
    {"field": 'age'},
    {"field": 'date'},
]
defaultColDef = {
      "filter": True,
      "floatingFilter": True,
      "editable": True,
}

app.layout = html.Div(
    [
        dag.AgGrid(
            id="grid-cell-data-types",
            columnDefs=columnDefs,
            rowData=df.to_dict("records"),
            defaultColDef=defaultColDef,
            dashGridOptions={"dataTypeDefinitions": {"function": "dataTypeDefinitions"}},
        ),
    ],
)

app.run(debug=True)



"""

dagfuncs.dataTypeDefinitions = {
      dateString: {
        baseDataType: 'dateString',
        extendsDataType: 'dateString',
        valueParser: (params) =>
          params.newValue != null &&
          params.newValue.match('\\d{2}/\\d{2}/\\d{4}')
            ? params.newValue
            : null,
        valueFormatter: (params) => (params.value == null ? '' : params.value),
        dataTypeMatcher: (value) =>
          typeof value === 'string' && !!value.match('\\d{2}/\\d{2}/\\d{4}'),
        dateParser: (value) => {
          if (value == null || value === '') {
            return undefined;
          }
          const dateParts = value.split('/');
          return dateParts.length === 3
            ? new Date(
                parseInt(dateParts[2]),
                parseInt(dateParts[1]) - 1,
                parseInt(dateParts[0])
              )
            : undefined;
        },
        dateFormatter: (value) => {
          if (value == null) {
            return undefined;
          }
          const date = String(value.getDate());
          const month = String(value.getMonth() + 1);
          return `${date.length === 1 ? '0' + date : date}/${
            month.length === 1 ? '0' + month : month
          }/${value.getFullYear()}`;
        },
      },
    };


"""


@AnnMarieW
Copy link
Collaborator

AnnMarieW commented Dec 4, 2023

Is the date object cell data type applicable in Dash? If so, how would you use it?

Update - We are recommending not to use this with Dash since the Date Object is not JSON compatible.

@AnnMarieW
Copy link
Collaborator

AnnMarieW commented Dec 7, 2023

Update - the issues described here have been fixed


There are a couple warning messages in the console in V31

AG Grid: Since v31, 'columnApi.getColumnState' is deprecated and moved to 'api.getColumnState'.

AG Grid: Since v31, 'columnApi.autoSizeAllColumns' is deprecated and moved to 'api.autoSizeAllColumns'.

I'm not sure which example is triggering this warning message:

AG Grid: Grid API function getColumnState() cannot be called as the grid has been destroyed.
It is recommended to remove local references to the grid api. Alternatively, check gridApi.isDestroyed() to avoid calling methods against a destroyed grid. To run logic when the grid is about to be destroyed use the gridPreDestroy event

… added a test for `gridApi?.isDestroyed()`
@BSd3v
Copy link
Collaborator Author

BSd3v commented Dec 11, 2023

There are a couple warning messages in the console in V31

AG Grid: Since v31, 'columnApi.getColumnState' is deprecated and moved to 'api.getColumnState'.

AG Grid: Since v31, 'columnApi.autoSizeAllColumns' is deprecated and moved to 'api.autoSizeAllColumns'.

I'm not sure which example is triggering this warning message:

AG Grid: Grid API function getColumnState() cannot be called as the grid has been destroyed. It is recommended to remove local references to the grid api. Alternatively, check gridApi.isDestroyed() to avoid calling methods against a destroyed grid. To run logic when the grid is about to be destroyed use the gridPreDestroy event

I think I got these working, please check whatever was throwing these warnings.

@AnnMarieW
Copy link
Collaborator

AnnMarieW commented Dec 29, 2023

The example posted above now works 🎉


Update - the issue described below is a bug and reported on the AG Grid repo. Should be fixed in the next release 🎉 ag-grid/ag-grid#7398 (comment)


Now I'm trying to make it work with various date formats. For example, if you wanted to create custom cell data types and assign it to a column, it doesn't seem to work.

Note that in the example below, the custom cell data type for the percentage works, but the date does not.


import dash_ag_grid as dag
from dash import Dash, html
import pandas as pd


app = Dash(__name__)

df = pd.read_csv(
    "https://raw.githubusercontent.com/plotly/datasets/master/ag-grid/olympic-winners.csv"
)

columnDefs = [
    {"field": 'athlete'},
    {"field": 'age', "cellDataType": "percentage"},
    {"field": 'date', "cellDataType": "dateString2"},
]
defaultColDef = {
      "filter": True,
      "floatingFilter": True,
      "editable": True,
}


app.layout = html.Div(
    [
        dag.AgGrid(
            id="grid-cell-data-types",
            columnDefs=columnDefs,
            rowData=df.to_dict("records"),
            defaultColDef=defaultColDef,
            dashGridOptions={"dataTypeDefinitions": {'function': 'dataTypeDefinitions'}},
        ),
    ],
)

app.run(debug=True, port=5000)

"""

dagfuncs.dataTypeDefinitions = {
      dateString2: {
        baseDataType: 'dateString',
        extendsDataType: 'dateString',
        valueParser: (params) =>
          params.newValue != null &&
          params.newValue.match('\\d{2}/\\d{2}/\\d{4}')
            ? params.newValue
            : null,
        valueFormatter: (params) => (params.value == null ? '' : params.value),
        dataTypeMatcher: (value) =>
          typeof value === 'string' && !!value.match('\\d{2}/\\d{2}/\\d{4}'),
        dateParser: (value) => {
          if (value == null || value === '') {
            return undefined;
          }
          const dateParts = value.split('/');
          return dateParts.length === 3
            ? new Date(
                parseInt(dateParts[2]),
                parseInt(dateParts[1]) - 1,
                parseInt(dateParts[0])
              )
            : undefined;
        },
        dateFormatter: (value) => {
          if (value == null) {
            return undefined;
          }
          const date = String(value.getDate());
          const month = String(value.getMonth() + 1);
          return `${date.length === 1 ? '0' + date : date}/${
            month.length === 1 ? '0' + month : month
          }/${value.getFullYear()}`;
        },
      },
      percentage: {
            extendsDataType: 'number',
            baseDataType: 'number',
            valueFormatter: params => params.value == null
                ? ''
                : `${Math.round(params.value * 100)}%`,
        }
    };





"""




@AnnMarieW
Copy link
Collaborator

AnnMarieW commented Jan 8, 2024

Update - Done 🎉

Could you please add:

import 'ag-grid-community/styles/ag-theme-quartz.css';

https://github.com/plotly/dash-ag-grid/blob/main/src/lib/fragments/AgGrid.react.js#L54

BSd3v and others added 7 commits January 8, 2024 12:18
Add textFormatter and textMatcher in COLUMN_MAYBE_FUNCTIONS
… bump-to-v30

# Conflicts:
#	src/lib/utils/propCategories.js
* converting looping through object to return a new object for `dataTypeDefinitions`
package.json Outdated
@@ -1,6 +1,6 @@
{
"name": "dash-ag-grid",
"version": "2.4.0",
"version": "31.0.1rc3",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer we not try to match the minor or patch version to AG Grid, just the major. That way we still have the flexibility to add features and fix bugs on our own schedule, we just have to hold breaking changes to align with upgrading the AG Grid major version. So the new release should be 31.0.0

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sdidier-dev - can you chime in here? This may affect how we link to the correct AG Grid version in the docs

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think we need to be in line about how we'll handle the dag version vs AG Grid version to know how to deal with the links to AG Grid docs: if they go to the last version of AG Grid docs or to an "archive" version of their docs.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be done in the docs itself, you cannot depend on our version, like @alexcjohnson, we need to have flexibility.

Maybe this becomes a map in the component, and we just update it there. Possibly a function that the user can run clientside as well that returns the underlying AG Grid version.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually that shouldn't be an issue for the links!
We took a look with Ann Marie, we're using the version of the ag grid package used in dag:
AG_grid_version = dag.package['dependencies']['ag-grid-community'].lstrip('^')
then compare it to the latest version of AG Grid coming from their github repo:
latest_tag_url = "https://api.github.com/repos/ag-grid/ag-grid/releases/latest"

So the version of dag can be anything that's should be fine for the links to AG Grid docs 🌈

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alexcjohnson,

What are your thoughts on being able to query the underlying grid version from the clientside? Or... could this just be done by searching the source code?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be handy have a utility -- maybe something like dag.get_grid_version()

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love the idea of a utility for this! Clientside seems most useful to me, because then you can run it as a viewer, not just an app developer, but we can do both if you like.

BSd3v and others added 5 commits February 1, 2024 12:44
Co-authored-by: Alex Johnson <alex@plot.ly>
…on, it will throw exception and alert the user for the error
…rst error, allowing it to check all 3 grid dependencies.
… same across the dependencies. This error will point to needing to disable the test if AG Grid becomes out of sync across the versions.
Copy link
Collaborator

@alexcjohnson alexcjohnson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💃 Let's do it!!

@BSd3v BSd3v merged commit a2b1334 into plotly:main Feb 1, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants